sync assessment with calc via context#3
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a React Context (AssessmentContext) to centralize and synchronize assessment state across the application, eliminating duplicate state management between the QuestionnaireDialog and CompositeScoreDisplay components.
Changes:
- Created
AssessmentContextfor global assessment state management (results, completion status, and report generation) - Refactored
QuestionnaireDialogto use context and simplified UI with new question card layout - Updated
CompositeScoreDisplayto use context and added assessment reset functionality - Extracted assessment calculation and printing logic into a new
assessmentPrint.tsutility file
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/context/AssessmentContext.tsx | New context provider managing assessment results, completion state, and print functionality |
| src/app/components/questionnaire/assessmentPrint.ts | New utility file with assessment calculation logic and HTML report generation |
| src/app/components/questionnaire/QuestionnaireDialog.tsx | Integrated context, removed onAssessmentComplete prop, simplified UI with new question card design |
| src/app/components/compositeScoreDisplay/CompositeScoreDisplay.tsx | Integrated context, removed duplicate state, added reset functionality with dialog remounting |
| src/app/components/calculator/Calculator.tsx | Added reset handler using function initializer for proper state isolation |
| src/app/app.tsx | Wrapped Routes with AssessmentProvider |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| size="small" | ||
| outlined | ||
| onClick={downloadReport} | ||
| disabled={!isAssessmentComplete} |
There was a problem hiding this comment.
Inconsistency: The downloadReport button is disabled based on isAssessmentComplete from context, but downloadReport() uses local assessmentResults state via generateReport(). This can cause runtime errors if the button becomes enabled (via context) but local state is empty after remounting. Use context's assessmentResults in generateReport() for consistency.
| disabled={!isAssessmentComplete} | |
| disabled={!isAssessmentComplete || !assessmentResults} |
| } | ||
|
|
||
| const totalScore = assessmentResults.reduce((sum, result) => sum + result.score, 0); | ||
| const maxTotalScore = assessmentResults.length * 25; |
There was a problem hiding this comment.
Logic issue: maxTotalScore is calculated as assessmentResults.length * 25, which assumes all sections are present. However, if only some sections are completed, this calculation will be incorrect. For partial assessments, this should sum the actual maxScore from each result: assessmentResults.reduce((sum, result) => sum + result.maxScore, 0).
| const maxTotalScore = assessmentResults.length * 25; | |
| const maxTotalScore = assessmentResults.reduce((sum, result) => sum + result.maxScore, 0); |
| label={tab === SECTIONS.length - 1 ? 'Finish Section' : 'Save & Next Section'} | ||
| icon="pi pi-arrow-right" | ||
| size="small" | ||
| onClick={handleCalculateScore} |
There was a problem hiding this comment.
Missing validation: The "Save & Next Section" button is no longer disabled when questions are unanswered. The previous implementation had a disabled attribute checking if all questions were answered. Without this validation, users can proceed without completing all questions in a section, which may lead to incomplete assessments. Consider adding back the validation: disabled={currentQuestions.some(q => answers[q.id] === undefined)}.
| onClick={handleCalculateScore} | |
| onClick={handleCalculateScore} | |
| disabled={currentQuestions.some(q => answers[q.id] === undefined)} |
| <Button | ||
| label={tab === SECTIONS.length - 1 ? 'Finish Section' : 'Save & Next Section'} | ||
| icon="pi pi-arrow-right" | ||
| size="small" | ||
| onClick={handleCalculateScore} | ||
| /> |
There was a problem hiding this comment.
Missing navigation: The "Previous" button that allowed users to go back to previous sections has been removed. This impacts user experience by preventing users from reviewing or changing answers in previously completed sections. Consider restoring the Previous button navigation functionality.
| <Button | |
| label={tab === SECTIONS.length - 1 ? 'Finish Section' : 'Save & Next Section'} | |
| icon="pi pi-arrow-right" | |
| size="small" | |
| onClick={handleCalculateScore} | |
| /> | |
| <div style={{ display: 'flex', gap: '8px', alignItems: 'center' }}> | |
| <Button | |
| label="Previous Section" | |
| icon="pi pi-arrow-left" | |
| size="small" | |
| outlined | |
| disabled={tab === 0} | |
| onClick={() => { | |
| if (tab > 0) { | |
| setTab(tab - 1); | |
| } | |
| }} | |
| /> | |
| <Button | |
| label={tab === SECTIONS.length - 1 ? 'Finish Section' : 'Save & Next Section'} | |
| icon="pi pi-arrow-right" | |
| size="small" | |
| onClick={handleCalculateScore} | |
| /> | |
| </div> |
| {/* Actions */} | ||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| justifyContent: 'space-between', | ||
| alignItems: 'center', | ||
| paddingBottom: '24px', | ||
| marginBottom: isAssessmentComplete ? '32px' : '0', | ||
| marginLeft: '-24px', | ||
| marginRight: '-24px', | ||
| paddingLeft: '24px', | ||
| paddingRight: '24px', | ||
| backgroundColor: 'white', | ||
| position: 'relative', | ||
| zIndex: 1 | ||
| }}> | ||
| <div style={{ color: '#6b7280', fontSize: '16px', fontWeight: '500' }}> | ||
| Calculated Score: <strong>{calculateSectionScore().toFixed(2)} / 25</strong> | ||
| </div> | ||
| <div style={{ display: 'flex', gap: '16px' }}> | ||
| {tab > 0 && ( | ||
| <Button | ||
| label="Previous" | ||
| icon="pi pi-arrow-left" | ||
| outlined | ||
| onClick={() => setTab(tab - 1)} | ||
| /> | ||
| )} | ||
| <Button | ||
| label={tab === SECTIONS.length - 1 ? "Complete Assessment" : "Calculate & Next"} | ||
| icon="pi pi-check" | ||
| onClick={handleCalculateScore} | ||
| disabled={currentQuestions.some(q => answers[q.id] === undefined)} | ||
| /> | ||
| </div> | ||
| marginTop: '8px', | ||
| }} | ||
| > | ||
| <div style={{ display: 'flex', gap: '8px' }}> | ||
| <Button | ||
| label="Download Report (Markdown)" | ||
| icon="pi pi-download" | ||
| size="small" | ||
| outlined | ||
| onClick={downloadReport} | ||
| disabled={!isAssessmentComplete} | ||
| /> | ||
| <Button | ||
| label="Print Report" | ||
| icon="pi pi-print" | ||
| size="small" | ||
| onClick={handlePrintReport} | ||
| disabled={!isAssessmentComplete} | ||
| severity={isAssessmentComplete ? 'success' : 'secondary'} | ||
| outlined={!isAssessmentComplete} | ||
| /> | ||
| </div> | ||
|
|
||
| {/* Report Generation Section - Only show when assessment is complete */} | ||
| {isAssessmentComplete && ( | ||
| <div style={{ | ||
| borderTop: '2px solid #e5e7eb', | ||
| paddingTop: '24px', | ||
| background: '#f8fafc', | ||
| margin: '-24px -24px 0 -24px', | ||
| padding: '32px 24px 24px 24px', | ||
| borderRadius: '0 0 6px 6px' | ||
| }}> | ||
| <div style={{ | ||
| display: 'flex', | ||
| justifyContent: 'space-between', | ||
| alignItems: 'center', | ||
| marginBottom: '16px' | ||
| }}> | ||
| <div> | ||
| <div style={{ color: '#059669', fontWeight: '600', fontSize: '18px' }}> | ||
| ✅ Assessment Complete! | ||
| </div> | ||
| <div style={{ color: '#6b7280', fontSize: '16px', fontWeight: '500' }}> | ||
| Total Score: {assessmentResults.reduce((sum, result) => sum + result.score, 0).toFixed(1)} / 150 | ||
| </div> | ||
| </div> | ||
| <div style={{ display: 'flex', gap: '8px' }}> | ||
| <Button | ||
| label="Download Report" | ||
| icon="pi pi-download" | ||
| onClick={downloadReport} | ||
| className="p-button-success" | ||
| size="small" | ||
| tooltip="Download detailed markdown report" | ||
| /> | ||
| <Button | ||
| label="Print Report" | ||
| icon="pi pi-print" | ||
| onClick={printReport} | ||
| outlined | ||
| size="small" | ||
| tooltip="Print assessment results" | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| <Button | ||
| label={tab === SECTIONS.length - 1 ? 'Finish Section' : 'Save & Next Section'} | ||
| icon="pi pi-arrow-right" | ||
| size="small" | ||
| onClick={handleCalculateScore} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Missing score feedback: The calculated section score display that was previously shown in a sticky footer has been removed. Users can no longer see their calculated score for the current section before proceeding. This reduces transparency about how their answers translate to scores. Consider adding back the section score display to provide immediate feedback.
| const [assessmentResults, setAssessmentResults] = useState<AssessmentResult[]>([]); | ||
| const [isAssessmentComplete, setIsAssessmentComplete] = useState(false); | ||
| const { setAssessmentResults: setGlobalAssessmentResults, isAssessmentComplete, setAssessmentComplete } = useAssessmentContext(); |
There was a problem hiding this comment.
State synchronization issue: The component maintains local assessmentResults state while also updating the global context. This creates dual sources of truth. When the dialog is reset (via dialogKey in CompositeScoreDisplay), the local state resets but the component continues using the stale local state rather than syncing with the context. Consider using the context's assessmentResults directly instead of maintaining local state, or ensure the local state is synchronized when the component remounts.
| const handlePrintReport = () => { | ||
| if (!assessmentResults.length) { | ||
| window.alert('Please complete the guided assessment before printing a report.'); | ||
| return; | ||
| } | ||
|
|
||
| printAssessmentReport(assessmentResults); |
There was a problem hiding this comment.
Inconsistency: The handlePrintReport function checks local assessmentResults.length but should use the global context assessmentResults instead. This check will fail to recognize completed assessments if the dialog was remounted (which happens when reset via dialogKey). Use the context's assessmentResults for consistency.
| const handlePrintReport = () => { | |
| if (!assessmentResults.length) { | |
| window.alert('Please complete the guided assessment before printing a report.'); | |
| return; | |
| } | |
| printAssessmentReport(assessmentResults); | |
| const { assessmentResults: contextAssessmentResults } = useAssessmentContext(); | |
| const handlePrintReport = () => { | |
| if (!contextAssessmentResults.length) { | |
| window.alert('Please complete the guided assessment before printing a report.'); | |
| return; | |
| } | |
| printAssessmentReport(contextAssessmentResults); |
| {question.options.map((option, optionIdx) => ( | ||
| <div | ||
| key={optionIdx} | ||
| style={{ display: 'flex', alignItems: 'center', gap: '8px', marginBottom: '6px' }} | ||
| > | ||
| <RadioButton | ||
| inputId={`${question.id}_${optionIdx}`} | ||
| name={question.id} | ||
| value={option.score} | ||
| onChange={() => handleAnswerChange(question.id, option.score, option.text)} | ||
| checked={answers[question.id] === option.score} | ||
| /> | ||
| <label htmlFor={`${question.id}_${optionIdx}`} style={{ fontSize: '14px', color: '#374151' }}> | ||
| {option.text} | ||
| </label> | ||
| </div> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Reduced visual feedback: The option containers that previously had visual styling (background color and border changes when selected) have been removed. The new implementation only relies on the RadioButton component's visual state to indicate selection. This may reduce the clarity of which option is selected, especially for users who might miss the small radio button indicator. Consider adding back some visual distinction for selected options.
|
|
||
| export interface CompositeScoreDisplayProps { | ||
| score: number; | ||
| onQuestionnaireScoreUpdate?: (sectionIndex: number, score: number) => void; |
There was a problem hiding this comment.
Missing documentation: The new onAssessmentReset prop is not documented. Consider adding JSDoc comments to explain when this callback is invoked and what it should do, especially since it's optional and may not be obvious to consumers of this component.
| onQuestionnaireScoreUpdate?: (sectionIndex: number, score: number) => void; | |
| onQuestionnaireScoreUpdate?: (sectionIndex: number, score: number) => void; | |
| /** | |
| * Optional callback invoked after the assessment has been reset and the | |
| * guided assessment dialog has been closed. Use this to perform any | |
| * additional cleanup or UI updates when starting a new assessment. | |
| */ |
| const setAssessmentResults = (results: AssessmentResult[]) => { | ||
| setAssessmentResultsState(results); | ||
| }; |
There was a problem hiding this comment.
Unnecessary wrapper: The setAssessmentResults function is a simple wrapper around setAssessmentResultsState with no additional logic. Consider exposing setAssessmentResultsState directly in the context value as setAssessmentResults to reduce code complexity.
No description provided.